fix(vueNodes): decrease default size of reroute nodes#8734
Open
christian-byrne wants to merge 8 commits intomainfrom
Open
fix(vueNodes): decrease default size of reroute nodes#8734christian-byrne wants to merge 8 commits intomainfrom
christian-byrne wants to merge 8 commits intomainfrom
Conversation
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ❌ 542 passed, 2 failed · 6 flaky❌ Failed Tests📊 Browser Reports
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDisables UI resizing for reroute nodes, tightens their computed size, exempts them from default min-width/min-height and certain padding in the Vue renderer, and adds unit and E2E tests verifying smaller sizing and absence of resize handles. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
📦 Bundle: 4.46 MB gzip 🟢 -258 BDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.03 MB (baseline 1.03 MB) • 🔴 +344 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.87 MB (baseline 7.87 MB) • 🔴 +38 BBundles that do not match a named category
Status: 52 added / 52 removed |
christian-byrne
added
the
preview-cpu
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
from
2e7d670 to
90bea80
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@browser_tests/tests/vueNodes/rerouteNodeSize.spec.ts`:
- Around line 68-69: The assertion is checking for zero resize handles but uses
.last(), which is misleading; update the locator usage by removing .last() so
that resizeHandle is obtained via
rerouteEl.locator('[role="button"][aria-label]') and then assert await
expect(resizeHandle).toHaveCount(0) (keep the existing selector fallback),
ensuring the test clearly asserts that no resize handle elements exist.
🧹 Nitpick comments (1)
browser_tests/tests/vueNodes/rerouteNodeSize.spec.ts (1)
16-20: Consider extracting the repeated reroute node lookup into a helper.The logic to find the reroute node ID via
page.evaluateis duplicated across all four tests. Extracting this into a helper function would reduce duplication and improve maintainability.♻️ Suggested helper extraction
async function getRerouteNodeId(page: Page): Promise<string | null> { return page.evaluate(() => { const graph = window.app!.graph! const rerouteNode = graph.nodes.find((n) => n.type === 'Reroute') return rerouteNode ? String(rerouteNode.id) : null }) }Then in each test:
const rerouteNodeId = await getRerouteNodeId(comfyPage.page) expect(rerouteNodeId).not.toBeNull()Also applies to: 33-41, 59-63, 75-79
dosubot
bot
added
the
size:L
Member
|
In litegraph mode the node is very hard to move as the clickable area for the slots takes up most of the node |
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
3 times, most recently
from
a88a5e6 to
9925d27
Compare
Contributor
Author
|
I simplified it 
|
Contributor
Author
|
Test here: https://pr-2369.testenvs.comfy.org/ |
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
from
be310cd to
a3cbc5a
Compare
Member
|
They still create big then shrink on first connect big.reroute.2.mp4 |
- Skip min-w-[225px] and min-h constraints for reroute nodes in Vue - Render bare NodeSlots without body wrapper for compact layout - Hide footer Button and resize handles for reroute nodes - All changes are Vue-only; no modifications to shared litegraph code - Add screenshot baseline test for visual regression Fixes #4704 Amp-Thread-ID: https://ampcode.com/threads/T-019c5893-c9d2-77cb-9e90-a9a0631220db
Reroute nodes without min-h caused ResizeObserver to measure a tiny DOM height. After removeNodeTitleHeight subtracted 30px the stored height became 0, making the node invisible when switching to litegraph mode. Use h-(--node-height) so the DOM height always matches computeSize() after the title-height normalization. Amp-Thread-ID: https://ampcode.com/threads/T-019c59fb-447e-7191-ba36-b85f723346e1
…d render RerouteNode inherited default LGraphNode size [140, 60] but never called setSize(computeSize()) to override it to [75, 26]. This caused reroute nodes to render at the large default size, then shrink on first connection. Amp-Thread-ID: https://ampcode.com/threads/T-019c9d18-bed1-710c-973e-94d2b3dcba6b
christian-byrne
force-pushed
the
fix/vue-nodes/reroute-default-size
branch
from
a79cbbf to
e78fec0
Compare
dosubot
bot
added
size:M
⚡ Performance Report
Raw data{
"timestamp": "2026-02-28T12:49:13.713Z",
"gitSha": "07277711ad18b700d17fdc7ebb620668533cc3e2",
"branch": "fix/vue-nodes/reroute-default-size",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2031.0910000000035,
"styleRecalcs": 123,
"styleRecalcDurationMs": 20.873999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 412.23199999999997,
"heapDeltaBytes": -3220324
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1913.8759999999877,
"styleRecalcs": 174,
"styleRecalcDurationMs": 51.77899999999999,
"layouts": 12,
"layoutDurationMs": 3.779,
"taskDurationMs": 864.888,
"heapDeltaBytes": -3494644
},
{
"name": "dom-widget-clipping",
"durationMs": 612.4849999999924,
"styleRecalcs": 46,
"styleRecalcDurationMs": 15.635000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.34599999999995,
"heapDeltaBytes": 7895480
}
]
} |
Contributor
Author
|
/update-playwright |
|
Updating Playwright Expectations |
Contributor
Author
|
/update-playwright |
christian-byrne
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the 225px minimum width constraint from reroute nodes so they render at their intended ~75×26px size.
Changes
min-w-[225px]CSS constraint and bottom padding applied to regular nodes.resizable: falseis set on the RerouteNode constructor to hide the resize handle. AnisRerouteNodecomputed inLGraphNode.vuegates these behaviors by checkingnodeData.type === "Reroute".Review Focus
type === "Reroute"(explicit) rather thantitleMode === NO_TITLE(semantic but too broad). See PR fix(vue-nodes): hide slot labels for reroute nodes with empty names #8574 as prior art for reroute-specific conditionals.LGraphNode.tspos/size setters but does not touchLGraphNode.vuetemplate or resize callback — no conflict expected.Fixes #4704
Screenshots (if applicable)
Reroute nodes now render at ~75px wide instead of being forced to 225px minimum.
┆Issue is synchronized with this Notion page by Unito